Conversation
968bf7e to
6f2e034
Compare
b3dfcdc to
0838a95
Compare
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
efa8386 to
ab1a74a
Compare
|
Now passing 1-gpu/node, 8 ranks PTG POTRF |
This comment was marked as resolved.
This comment was marked as resolved.
cd7c475 to
3bab2d5
Compare
eb5c782 to
3e0cb38
Compare
|
I think we need to create a CI test that targets gpu_nvidia and issues the job to that runner, correct? |
|
Failure in See further discussion in #671 (comment)
|
| * from the data and eventually release their memory. | ||
| */ | ||
| parsec_data_copy_detach(data, copy, copy->device_index); | ||
| zone_free((zone_malloc_t *)copy->arena_chunk, copy->device_private); |
There was a problem hiding this comment.
Has this been checked for per-tile allocated data? The release of that memory is different and should not go into the zone allocator.
| gpu_device->super.device_index, gpu_device->super.name, original->key, (void*)gpu_copy->device_private); | ||
| } | ||
| assert(0 != (gpu_copy->flags & PARSEC_DATA_FLAG_PARSEC_OWNED) ); | ||
| assert(0 != (gpu_copy->flags & PARSEC_DATA_FLAG_PARSEC_OWNED)); |
There was a problem hiding this comment.
I know this has been here before, but: how should we handle copies that are not owned by parsec? They still seem to end up in the LRU even if they are only managed, not owned.
This comment was marked as resolved.
This comment was marked as resolved.
|
I merged with master but apparently I missed a defect in erroneous cases printouts, that causes the CI failures. |
parsec/mca/device/device_gpu.c
Outdated
| gpu_copy->coherency_state = PARSEC_DATA_COHERENCY_SHARED; | ||
| assert(PARSEC_DATA_STATUS_UNDER_TRANSFER == cpu_copy->data_transfer_status); | ||
| cpu_copy->data_transfer_status = PARSEC_DATA_STATUS_COMPLETE_TRANSFER; | ||
| if( 0 == (parsec_mpi_allow_gpu_memory_communications & PARSEC_RUNTIME_SEND_GPU_MEMORY) ) { |
There was a problem hiding this comment.
Followup to #671 (comment)
Issue with stress and friends is stemming from here: when we pushout and we have a successor task, the successor task must receive the cpu copy as input (otherwise it will reference a gpu_copy that is now in the read LRU).
A fix would need to distinguish between the case where we pushout to satisfy a communication and we pushout to satisfy the input of a CPU-only task.
There was a problem hiding this comment.
if a pushout is requested then we should always pass back the CPU copy (after it was updated). I need to understand the case yo are describing here. Is there a simple reproducer I can play with ?
There was a problem hiding this comment.
The reproducer is the stress CI tester
ci failure due to using data_in uninitialized in new context nc during ontask, introduced by changes to task_snprintf in 108b778 |
This comment was marked as resolved.
This comment was marked as resolved.
|
I see dangling copies on the device. This might just require a fix in the cleanup code (ignoring data that only has the device copy): |
This comment was marked as resolved.
This comment was marked as resolved.
Name the data_t allocated for temporaries allowing developers to track them through the execution. Add the keys to all outputs (tasks and copies). Signed-off-by: George Bosilca <gbosilca@nvidia.com>
This warning is overcautious now that we have GPU-only copies created from the network. Ideally we would find a way to discriminate between the real leakages from the application and these temporaries being reclaimed. |
|
The original associated with these device owned copies should not have a valid dc ? |
|
Here is what I think happens in the
data.data = this_task->data._f_A.data_out;
I don't understand the reshape code and I was hoping to never have to touch it. I suspect that the reshape code was not designed with GPUs in mind but I could be wrong. Will need some help digging through this and figuring out
|
5555bed to
76bc47f
Compare
|
For completeness let me try to describe what the issue was. In fact @abouteiller pointed to it earlier in the comments, but it went unnoticed. To clarify: in order to allow sending to remote successors directly from the GPU, this PR made a drastic change by passing to successors the data copy the task has worked on, instead of the CPU version. However, it did not allowed the two versions to drift apart, it updated the CPU version to mirror the device copy (which is great as the two are now equivalent). But by passing the device data copy it alters the way we track the reference count for copies (propagate the output copies to successors to trigger their ref count update before releasing the refcounts on the current task input). In most cases, it works just fine because the CPU copies has other references to it, at least because it belongs to a data collection. But in this test we are using a NEW data copy, one that is not associated with any data collection but instead specifically created for the chain of GEMM task. The first time we create this data, we also create the CPU mirror for it, but that mirrored copy has a refcount of 1 because it is only referenced once (ir does not belong to a data collection). So, when the first GEMM task in the chain passes the GPU copy to its successor and releases it's own input copies, it releases the only reference to the CPU copy allowing the runtime to dispose of the copy. This is bad because now we have a valid device copy passing around tasks that does not have a backend CPU copy. |
76bc47f to
c1a0455
Compare
Signed-off-by: George Bosilca <gbosilca@nvidia.com>
This allows to check if the data can be send and received directly to and from GPU buffers. Signed-off-by: George Bosilca <gbosilca@nvidia.com>
This is a multi-part patch that allows the CPU to prepare a data copy mapped onto a device. 1. The first question is how is such a device selected ? The allocation of such a copy happen way before the scheduler is invoked for a task, in fact before the task is even ready. Thus, we need to decide on the location of this copy only based on some static information, such as the task affinity. Therefore, this approach only works for owner-compute type of tasks, where the task will be executed on the device that owns the data used for the task affinity. 2. Pass the correct data copy across the entire system, instead of falling back to data copy of the device 0 (CPU memory) Add a configure option to enable GPU-aware communications. Signed-off-by: George Bosilca <gbosilca@nvidia.com>
Name the data_t allocated for temporaries allowing developers to track them through the execution. Add the keys to all outputs (tasks and copies). Signed-off-by: George Bosilca <gbosilca@nvidia.com>
Signed-off-by: George Bosilca <gbosilca@nvidia.com>
copy if we are passed-in a GPU copy, and we need to retain/release the copies that we are swapping
…ut-only flows, for which checking if they are control flows segfaults
Mismatch in printf type sizes might lead to segmentation faults. Signed-off-by: Joseph Schuchart <joseph.schuchart@stonybrook.edu>
Signed-off-by: George Bosilca <gbosilca@nvidia.com>
Add missing dependency library. Signed-off-by: George Bosilca <gbosilca@nvidia.com>
0a5fdde to
f0dd212
Compare
This disable the communications from GPU memory, but it is necessary for a proper tracking of reference counts on the data copies. At this point I dont think we can fiddle with the code to allow for device copy propagation, too many corner cases to be able to be workable. Instead, we should rethink the entire data copy framework, and allow tasks to fetch their inputs from devices as needed. Signed-off-by: George Bosilca <gbosilca@nvidia.com>
I'm not sure I understand how this worked so far, because the temporary stack-based task we use to prepare the successors is barely initialized, not enough to call parsec_task_snprintf on it. The least info we need to set is to NULLify the data_in for all task classs flows. Signed-off-by: George Bosilca <gbosilca@nvidia.com>
f0dd212 to
ae5581f
Compare
Add support for sending and receiving the data directly from and to devices. There are few caveats (noted on the commit log).
Note: because it includes the
spanrenaming, this PR changes teh public API and will need to bump version to 5.xThe allocation of such a copy happen way before the scheduler is invoked
for a task, in fact before the task is even ready. Thus, we need to
decide on the location of this copy only based on some static
information, such as the task affinity. Therefore, this approach only
works for owner-compute type of tasks, where the task will be executed
on the device that owns the data used for the task affinity.
falling back to data copy of the device 0 (CPU memory)
TODOs
scheduling.c:157: int __parsec_execute(parsec_execution_stream_t *, parsec_task_t *): Assertion NULL != copy->original && NULL != copy->original->device_copies[0]'device_gpu.c:2470: int parsec_device_kernel_epilog(parsec_device_gpu_module_t *, parsec_gpu_task_t *): Assertion PARSEC_DATA_STATUS_UNDER _TRANSFER == cpu_copy->data_transfer_status' failed.